Skip to content

Fix unconditional retrieval loading for non-retrieval settings - #4

Open
Samar-Khalid wants to merge 2 commits into
beaverbench:mainfrom
Samar-Khalid:fix/retrieval-conditional
Open

Fix unconditional retrieval loading for non-retrieval settings#4
Samar-Khalid wants to merge 2 commits into
beaverbench:mainfrom
Samar-Khalid:fix/retrieval-conditional

Conversation

@Samar-Khalid

@Samar-Khalid Samar-Khalid commented Jul 30, 2026

Copy link
Copy Markdown

Description

This PR fixes a bug where retrieval tables were loaded unconditionally, even for preprocessing settings that use gold tables.

Problem

For preprocessing options that do not require retrieval, the pipeline still called get_retrieved_tables(), causing unnecessary assertions and failures when retrieval files were unavailable.

Root Cause

get_retrieved_tables() was executed before checking preprocessing_option, causing retrieval-related logic to run for non-retrieval settings.

Changes

  • Moved retrieval loading inside the appropriate conditional block.
  • Applied the same fix across all three affected pipelines:
    • ReFoRCE converter
    • DIN-SQL preprocessing
    • DAIL-SQL preprocessing

Testing

Added regression tests covering:

  • Non-retrieval settings running successfully without retrieval files.
  • Retrieval settings still enforcing retrieval requirements.

The existing retrieval behavior remains unchanged for retrieval-based settings.

All tests passed successfully.

Samar added 2 commits July 30, 2026 05:17
Setting 1 (option=2) and Setting 2 (option=3) do not require table
retrieval files, but get_retrieved_tables() was called unconditionally,
causing an AssertionError when retrieval files were absent.

This fix moves the get_retrieved_tables() call inside the
preprocessing_option == 1 guard, so it is only invoked when
retrieval is actually needed (Setting 0).

Added test verifying that preprocessing_option 2 and 3 succeed
without retrieval files.
- Same fix as the ReFoRCE converter: moved get_retrieved_tables()
  inside the option == 1 guard for DIN-SQL and DAIL-SQL.
- Updated regression test to cover all three pipelines.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant